Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Xcode7 compile error #147

Merged
merged 1 commit into from
Sep 29, 2015
Merged

Fix Xcode7 compile error #147

merged 1 commit into from
Sep 29, 2015

Conversation

robotbear2077
Copy link
Contributor

Absolute value function 'fabsf' given an argument of type 'double' but has parameter of type 'float' which may cause truncation of value.

…gument of type 'double' but has parameter of type 'float' which may cause truncation of value
@rhcad
Copy link
Collaborator

rhcad commented Sep 23, 2015

I don't think so.

The right fix may be int segments = ceilf(fabsf(thetaArc / ((float)M_PI_2 + 0.001f))) rather than ceilf(fabs(thetaArc / (M_PI_2 + 0.001f))). Because M_PI_2 is a double value and a double value will pass to the function ceilf in your PR.

Thank you!

@robotbear2077
Copy link
Contributor Author

@rhcad , thanks for your quick reply.

Sorry I didn't think much more then, my fix just takes the xcode's auto-fix suggestion, and the compiler seems happy with passing double value to ceilf, since you just want the integer part, so may be the precision doesn't matter here with ceilf?

Your suggestion is good for type consistency, but I don't think downcast the precision of M_PI_2 is a good choice, after all, precision does matter when it comes to fabs and fabsf, and the compiler did whining about it.

rhcad added a commit that referenced this pull request Sep 29, 2015
@rhcad rhcad merged commit 2def9e4 into sprang:develop Sep 29, 2015
@rhcad
Copy link
Collaborator

rhcad commented Sep 29, 2015

I think you are write. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants